Fix #374 add connector support #401
Conversation
🦋 Changeset detectedLatest commit: 47b7318 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| (| { | ||
| serverLabel: string; | ||
| serverUrl?: string; | ||
| authorization?: string; | ||
| headers?: Record<string, string>; | ||
| } | ||
| : { | ||
| type: 'mcp', | ||
| server_label: options.serverLabel, | ||
| server_url: options.serverUrl, | ||
| allowed_tools: toMcpAllowedToolsFilter(options.allowedTools), | ||
| headers: options.headers, | ||
| require_approval: | ||
| typeof options.requireApproval === 'string' | ||
| ? 'always' | ||
| : buildRequireApproval(options.requireApproval), | ||
| on_approval: options.onApproval, | ||
| }; | ||
| return { | ||
| type: 'hosted_tool', | ||
| name: 'hosted_mcp', | ||
| providerData, | ||
| }; | ||
| // OpenAI Connector | ||
| | { | ||
| serverLabel: string; | ||
| connectorId: string; | ||
| authorization?: string; | ||
| headers?: Record<string, string>; | ||
| } | ||
| ) & | ||
| ( | ||
| | { requireApproval?: never } | ||
| | { requireApproval: 'never' } | ||
| | { | ||
| requireApproval: | ||
| | 'always' | ||
| | { | ||
| never?: { toolNames: string[] }; | ||
| always?: { toolNames: string[] }; | ||
| }; | ||
| onApproval?: HostedMCPApprovalFunction<Context>; | ||
| } | ||
| ), |
There was a problem hiding this comment.
[P1] Require either serverUrl or connectorId for hosted MCP tools
The new overload for hostedMcpTool makes serverUrl optional in both branches while connectorId is only required in the connector branch. Because the runtime dispatch simply checks 'serverUrl' in options and 'connectorId' in options, it is now possible to call hostedMcpTool({ serverLabel: 'foo' }) without either value; this compiles and returns provider data that lacks both server_url and connector_id. When the model later tries to invoke the tool, OpenAI will reject the call because no endpoint or connector is specified, whereas the previous signature enforced the required field at compile time. Consider keeping serverUrl mandatory for non‑connector usage and validating that one of the two identifiers is provided so misconfigurations are caught early.
Useful? React with 👍 / 👎.
This pull request resolves #374